Positron: support custom providers - #71
Merged
Merged
Conversation
Two changes in ai-credentials, both needed so a host can accept named providers.custom.<name> entries. Built-in behaviour is unchanged. Read the provider map lazily. createPositronBackend took a ProviderMap table and read it once at construction, including a prebuilt reverse index for credential-change events. Custom entry ids are user-chosen and come and go while the process runs, so anything captured at construction silently stops resolving, and stops firing change events, for entries added later. The option becomes a getter and the reverse lookup is a scan over roughly twenty entries on auth session changes. Tell the CredentialConfig readers which provider they're answering for. The structured readers took no argument at all, so the only implementation read a hardcoded bedrock / snowflake-cortex / databricks connection: a custom type: "aws" entry inherited bedrock's region, and a type: "snowflake" entry never reached the Cortex URL path because a user-chosen name is not the string "snowflake-cortex". All five readers now take a CredentialConfigTarget carrying both the provider id and the configKey, and shapeCredentials takes the provider id and builds the target once. Both fields are needed, and only providerId identifies a provider. A configKey is derived (CONFIG_KEY_OVERRIDES maps snowflake-cortex to "snowflake") and therefore not unique: only built-in provider ids are reserved from custom entry names, so an entry named "snowflake" collides with the built-in's derived key and one of the two would get the other's connection. configKey stays for settings-backed adapters reading authentication.<configKey>.*. For structured base-URL derivation, AuthProviderMapping gains an optional structuredBaseUrl. Built-ins keep resolving through a module-local table keyed on auth provider id, so a host that hand-builds mappings for built-ins doesn't lose its derivation by leaving the field off; a custom entry declares it, because nothing about its id can imply it.
…gnal
`getSession(id, …, { silent: true })` blocks for several seconds before
rejecting with "Timed out waiting for authentication provider '<id>' to
register", and the backend caches that verdict for the process lifetime so the
wait isn't re-paid on every silent lookup. A session change clears the verdict,
since it means the provider registered.
But the provider can register *inside* that multi-second window. Then the
event's `delete` runs first and the rejection's `add` after it, so the verdict
sticks with no further event left to clear it: every later silent lookup takes
the fast path and the provider is permanently unresolvable until the window
reloads.
Count session changes per auth provider id and only cache the verdict if the
count hasn't moved since the lookup started.
This is the ordering a user hits with a `providers.custom` entry, where the
entry and its auth provider both appear at once: added mid-session, or loaded at
startup alongside the auth extension's registration.
Member
Author
|
opened a couple issues that came up while working on this: |
The snowflake branch of the apikey case read only host and account, then broke past the default branch that reads getBaseUrl, so an entry carrying a flat baseUrl resolved to no endpoint at all. Both shapes come from the same form. Standalone's Add-custom-provider writes a flat baseUrl in custom-URL mode and structured host/account otherwise, into the same providers.json, so honouring only one of them breaks half the entries that UI can create. The flat form is also the only one that can express a Cortex path other than /api/v2/cortex/v1, and the Node hosts already resolve conn.baseUrl ?? derive(conn). Structured still wins, so a stale URL can't shadow a host or account and nothing that resolved before changes. Built-in snowflake-cortex is unaffected either way: its flat baseUrl lives in the credential store, not the providers.json connection block, so the catalog never has one for it.
sharon-wang
force-pushed
the
positron-custom-providers
branch
from
August 21, 2026 20:09
a33c17f to
785f09a
Compare
sharon-wang
force-pushed
the
positron-custom-providers
branch
from
August 24, 2026 13:58
28208f6 to
99f9e22
Compare
`process.send("lock-released", () => resolve())` relies on the short
send(message, callback) overload, which @types/node only grew in 22.19.
It typechecks here because ai-lib installs 22.19.21 of its own, but
consumers that build these sources against a hoisted older copy get
TS2345: '() => void' is not assignable to 'SendHandle'.
The parent assistant repo hoists 22.18.13, so every CI job there that
runs build:bridge fails on this file. It isn't excluded from the build
either, since the tsconfig only excludes *.test.ts and this is a helper.
Passing sendHandle and options explicitly hits the long overload, which
has been stable across both versions.
sharon-wang
marked this pull request as ready for review
August 24, 2026 19:34
The Node catalog paths resolve conn.baseUrl ?? deriveSnowflakeBaseUrl(conn), so when a providers.json entry carries both leaves the flat URL wins there while this shaper preferred host/account — the same config could route Positron and Node products to different endpoints. Align on the Node precedence: an explicit flat baseUrl (the only shape that can express a non-standard Cortex path) wins, then host, then account.
The custom-entry tests registered one auth provider per entry (authProviderId === entry id, empty scopes), but the Positron host shares a single positron-custom-provider across all entries and identifies each by a [entryId] scope. With providerId === authProviderId the tests could not catch shaping or session lookup keyed on the auth-provider id instead of the logical entry id. Rework the fixtures to the real shape and assert scoped session lookup, per-entry baseUrls, and the shared provider's change-event fan-out to every entry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two API changes in
ai-credentialsso a host can resolve credentials forproviders.customentries: entries the user names themselves, which come and go while the process is running. Nothing changes for built-in providers.For posit-dev/positron#13823, posit-dev/positron#12747, posit-dev/positron#14141. Consumed by posit-dev/positron#15675 and posit-dev/assistant#2170.
Merge this first, then bump the ai-lib submodule in posit-dev/assistant#2170.
What changes
1.
PositronBackendtakes a provider-map getter instead of a tablecreatePositronBackend({ providerMap })used to take aProviderMapread once at construction. It now takes() => ProviderMap.Built-in providers are a fixed table, so reading it once was fine. Custom entries aren't: the ids are the user's chosen names, and adding, deleting, or disabling one in
providers.jsonchanges the map mid-session. With a captured table, anything added after the backend was built resolves to nothing and never fires a credential-change event, with no signal saying why.Two things follow from that:
getCredentialslooks its mapping up per call, and the auth-provider-id → provider-id reverse lookup happens per session-change event rather than from an index built at construction.2. A registration that lands mid-lookup can't poison the "not registered" verdict
Existing behaviour: when
getSessionrejects with "provider not registered", the backend remembers that and skips future silent lookups, because the rejection takes several seconds to arrive.That window is long enough for the provider to register while a lookup is still in the air. The
onDidChangeSessionsevent lands first and clears the verdict, then the stale rejection lands and re-installs it. The provider is now unresolvable for the rest of the process and no further event is coming to fix it. That is exactly the ordering you hit when a custom entry is added, or first loaded, at the same time its auth provider registers.Fix: count registration signals per auth provider. A lookup records the count it started under, and only caches its verdict if the count hasn't moved.
3.
CredentialConfigreads now say which provider they are forgetBaseUrlandgetCustomHeaderstook a bareconfigKeystring;getAws,getSnowflake, andgetDatabrickstook no arguments at all. All five now takeCredentialConfigTarget = { providerId, configKey }, andshapeCredentialsgains a leadingproviderId.Neither old form can name a custom entry:
configKeyis a settings namespace, not an identity, and it isn't unique.snowflake-cortexderives the configKeysnowflake, which is itself a legal custom entry name, so a reader keying on configKey hands a custom entry the built-in's connection.bedrockcould be AWS. A customtype: "aws"entry would inherit bedrock's region.providerIdcan't collide, because custom entry names reserve every built-in id (enforced on the Positron side). Catalog-backed adapters answer fromproviderId; settings-backed adapters keep readingauthentication.<configKey>.*. It's one object rather than two string parameters so the two can't be transposed silently.4.
structuredBaseUrlon the mappingSome apikey providers build their base URL from structured connection fields instead of a flat
baseUrl: Snowflake from host or account, Databricks from workspace host. Shaping picked those out withauthProviderId === "snowflake-cortex", which a user-named entry never matches, so atype: "snowflake"custom entry quietly fell through to the flat-baseUrl path.AuthProviderMappinggets an optionalstructuredBaseUrl: "snowflake" | "databricks". Built-ins keep their derivation through a small table keyed by auth provider id, so hosts that hand-build mappings for built-ins don't need updating.One deliberate behaviour change here: Snowflake now falls back to a flat
baseUrlwhen there is no host or account. Structured fields still win, so a stale URL can't shadow them, but the flat form has to resolve too. It's what standalone's Add-custom-provider form writes in custom-URL mode, it's the only shape that can express a non-standard Cortex path, and the other hosts already honour it (conn.baseUrl ?? derive(conn)).Callers
Both are breaking signature changes. The consumer PRs land the updates and should merge alongside this one:
CredentialConfigadapter and thecreatePositronBackendcallCredentialConfig, plus a mapping per enabled custom entryTests
PositronBackend:shapeCredentials:baseUrl, and a flatbaseUrlresolves when there are no structured fieldsbaseUrlsnowflakeisn't confused with the built-insnowflake-cortex